test(HTMLRewriter): count live allocator blocks for the handler leak on release builds - #44377

…ng the streams of a collected rewrite (#43379)

### Problem
- `element.onEndTag(fn)` makes `fn` a GC root (`ProtectedJSValue` in
`EndTagHandler`, `src/runtime/api/html_rewriter.rs`). If `fn` reaches
the output `Response` and the rewrite stops early, 200 of 200 Responses
stay.
- Main also uses the streams of a collected rewrite, in four places.
`cancel_from_output()` and `abandon_suspension()` close a dead JS input
(`SEGV` in `JSReadStreamIntoSinkOperation::result()`, or `ASSERTION
FAILED: status() == Status::Pending`). `abandon_suspension()` writes to
a freed output (`heap-use-after-free` in `ByteStream::on_data`).
`Bun.ModuleGraph.dispose()` cancels a stream source that is dead, or
that is freed under the call (a segfault on a release build with no
options). The leak hid the first.

### Fix
- `onEndTag` callbacks live in a JS array in `endTagHandlers`, a new
visited slot of the transform cell. The lol-html handler keeps an index.
- `cancel_from_output()` cuts the output's edge to the transform cell
last, as `fail()` and `finish()` already do.
- The pipe's reference to its own cell is a `JsRef` instead of a bare
`JSValue`, so a dead, unswept cell reads as `None`.
- The two entries that nothing reachable makes, the
`abandon_suspension()` task and `NewSource`'s `on_abort` (`dispose()`),
hold the wrapper that they read for the call, and leave a collected one
to its finalizer.
- Verified: 23 new tests (`test/js/workerd/html-rewriter-leak.test.ts`,
`module-graph-gc.test.ts`). 11 fail on a debug build of main, 13 on
release ASAN. Other suites: Notes. Self-reviewed: 3 concerns raised, 3
addressed.

### Background
- `gcProtect` makes a value a GC root, so a cycle through it is never
garbage. A visited slot is an edge that the collector traces.
- The transform cell (`HTMLRewriterTransform`) keeps the input stream of
one `transform()` alive. The native pipe holds that stream as a raw
pointer.
- Considered the cell in the `PipePin` guard of every entry point (an
earlier version of this PR), and a `Strong` on the input (a root per
rewrite). Notes say why not.

### Downsides
- `Element` grows from 40 to 48 bytes, `RewriterPipe` from 272 to 304,
the release binary by 8,960 bytes.
- A handler without `onEndTag()` costs the same within noise (150 ns per
element, base-to-base difference up to 1.2 ns). With it, 41 to 48 ns
less.
- Two leaks without `onEndTag()` remain (Notes).

<details><summary>Notes</summary>

**Report.** There is no GitHub issue. #43210 measured the leak and left
it for a follow-up, and its review asked for it:
#43210 (comment). This
is the last `ProtectedJSValue` in `html_rewriter.rs`.

**Why a visited slot.** `REVIEW.md` ("Root or copy every JSValue held
beyond the current call") asks for WriteBarrier members declared in
`.classes.ts`, and keeps `protect` for a justified self-keepalive.
#43210 applied that to the `handlers` slot. The third shape below also
needs it: a rewrite whose input never ends reaches no terminal state, so
only collection of the transform cell can free it, and a root prevents
that collection.

**Repro of the leak.** `MODE` is `ok`, `throw` or `cancel`. The `</div>`
end tag never arrives.

```js
const { heapStats } = require("bun:jsc");
let fired = 0;
const fr = new FinalizationRegistry(() => fired++);
const N = 200;
const mode = process.env.MODE ?? "throw";
async function once() {
  const holder = {};
  const rw = new HTMLRewriter()
    .on("div", { element(el) { el.onEndTag(() => holder.res); } })
    .on("p", { element() { if (mode === "throw") throw new Error("boom"); } });
  let ctrl;
  const res = rw.transform(new Response(new ReadableStream({ start(c) { ctrl = c; } })));
  holder.res = res;
  fr.register(res, 0);
  const chunk = new TextEncoder().encode("<div><p>x</p>");
  if (mode === "cancel") {
    const reader = res.body.getReader();
    ctrl.enqueue(chunk);
    await reader.read();
    await reader.cancel();
  } else {
    ctrl.enqueue(chunk);
    ctrl.close();
    try { await res.text(); } catch {}
  }
}
for (let i = 0; i < N; i++) await once();
for (let i = 0; i < 6; i++) { Bun.gc(true); await Bun.sleep(20); }
console.log(mode, "retained", N - fired, "of", N, "protected Function", heapStats().protectedObjectTypeCounts.Function ?? 0);
```

**Measurements of the leak** (debug builds, retained Responses and
protected Functions of 200, graphs of 15):

| shape | main (367d939) | this PR |
| --- | --- | --- |
| rewrite completes | 0, 0 | 0, 0 |
| a later handler throws, the body is read | 200, 200 | 0, 0 |
| the output reader cancels | 200, 200 | 0, 0 |
| the input never ends, all dropped, nothing pending on the output |
200, 200 | 0, 0 |
| disposed `Bun.ModuleGraph` whose code left such a rewrite | 15 graphs
| 1 graph |
| the same graph code without `onEndTag()` (control) | 1 graph | 1 graph
|

Release builds of the merge base 37471e5 and of this PR give the same
result for `throw` and `cancel`: 200 of 200, and 0 of 200. On main the
heap snapshot of the new ModuleGraph test names the retainer:
`root(ProtectedValues) Function -> JSLexicalEnvironment ->
JSModuleEnvironment -> JSLexicalEnvironment -Variable:moduleGraph->
ModuleGraph`.

**The dead input, case 1: `cancel_from_output()`.** An earlier version
of this PR failed on the x64 ASAN lane: the new test "when the output
reader is cancelled" aborted with `ASSERTION FAILED: status() ==
Status::Pending`. The cause is on main. `cancel_from_output()` starts
with `detach_output()`, which clears the `owner` slot of the output
stream. If script holds only the reader, that slot was the last GC path
to the transform cell. The next step allocates the abort reason, so a
collection can finish there. `detach_input_source()` then closes the
sink controller of the JS input through `SourceHandle::JSController`, a
raw pointer to a cell that only the transform cell kept alive. On main
the protected `onEndTag` callback kept such a rewrite alive forever, so
the window was closed for exactly the rewrites that the leak tests make.

Main has the bug with no `onEndTag()` call. This script requests a
collection before each `reader.cancel()`:

```js
const encoder = new TextEncoder();
const N = 200;
let cancelled = 0;
for (let i = 0; i < N; i++) {
  let controller;
  const input = new ReadableStream({ start: c => void (controller = c), cancel() { cancelled++; } });
  const reader = new HTMLRewriter().on("div", { element() {} }).transform(new Response(input)).body.getReader();
  controller.enqueue(encoder.encode("<div><p>x</p>"));
  controller = undefined;
  if ((await reader.read()).done) throw new Error("done early");
  Bun.gc(false);
  await reader.cancel();
}
Bun.gc(true);
console.log(JSON.stringify({ rewrites: N, cancelled }));
```

| release ASAN build | main | this PR |
| --- | --- | --- |
| plain run, 3 runs | `cancelled` is 195, 194, 200 of 200 | 200 of 200 |
| `BUN_JSC_collectContinuously=1` | `SEGV on unknown address
0x000000000010` | 200 of 200 |

The stack of the SEGV: `JSC::ClassInfo::isSubClassOf` <
`JSReadStreamIntoSinkOperation::result()` <
`Bun::WebStreams::rsisFinish` <
`jsWebStreamsHandler_onReadStreamIntoSinkClose` <
`JSC::runInternalMicrotask`. The failure needs a collection that
finishes between `detach_output()` and the close of the input. It did
not occur on a debug build or on a release build without ASAN. The
regression test runs the script under `BUN_JSC_collectContinuously=1` on
release builds (not on Windows, where that option is very slow), so it
fails on main only on the release ASAN lanes.

**The fix for case 1.** `detach_output()` moves to the end of
`cancel_from_output()`, next to `release_input_roots()`. The reader that
cancels then reaches the cell through the `owner` slot until the input
is closed. This is the order of `fail()` and `finish()`, and the rule
that the doc comment of `release_input_roots()` already states for the
input's edges. Alternatives:
- Keep the cell on the stack in the `PipePin` of `write`,
`end_from_stream`, `resume`, `cancel_from_output` and `fail` (an earlier
version of this PR). Each of those is entered by a peer whose own cell
has an internal edge to the transform cell (`owner`, `sinkOwner`, the
Response's `transform` slot, the context of a promise reaction). The
guard mattered where the pipe itself cut that edge too early (this
case), and where the first caller of the chain held nothing (cases 3 and
4). It does nothing for a cell that is already dead at the entry, so
that version still crashes in case 4. It also made `EnsureStillAlive` a
struct field. Everywhere else in the tree it is a local.
- A `Strong` on the controller in `SourceHandle::JSController`. That is
one more root per rewrite with a JS input, and a root is what made the
leak.

**The dead input, case 2: `abandon_suspension()`.** A review of this PR
found it. It is on main too. A handler returns a promise that never
settles, and script drops everything. The collector then frees the
promise, and the destructor of its native context queues
`abandon_suspension`. That task asked `cell.is_cell()` to learn if the
transform cell is alive. The pipe's `cell` field was a bare `JSValue`
that the cell's finalizer clears, and the finalizer runs at the sweep. A
cell that is dead but not swept yet still passes `is_cell()`, so the
task called `fail()`, and `fail()` closed the dead sink controller. That
field is a hand-written weak reference. `JsRef` is the one the rest of
the runtime uses for a native object's own wrapper (the stream sources
next to this pipe do), and since #39334 its `try_get()` gives `None` for
a dead, unswept cell, which is the answer of `JSC::Weak::get()`. The
field is now a `JsCell<JsRef>`, and every read of it goes through
`try_get()`. The task clears the input and output handles without a call
into them when that is `None`, as it already did for a swept cell.

```js
const N = 50;
const encoder = new TextEncoder();
const tick = () => new Promise(resolve => setImmediate(resolve));
let cancelled = 0;
for (let i = 0; i < N; i++) {
  let controller;
  const input = new ReadableStream({ start: c => void (controller = c), cancel: () => void cancelled++ });
  new HTMLRewriter().on("p", { element: () => new Promise(() => {}) }).transform(new Response(input));
  controller.enqueue(encoder.encode("<p>x</p>"));
}
for (let i = 0; i < 5; i++) await tick();
for (let round = 0; round < 10; round++) {
  Bun.gc(false);
  const junk = [];
  for (let j = 0; j < 500; j++) junk.push({ j });
  await tick();
}
process.stdout.write(JSON.stringify({ rewrites: N, cancelled }));
```

Run it as a file. A release build of main exits with `panic(main
thread): Segmentation fault at address 0x0` in 10 of 10 runs. A debug
build of main stops at `ASSERTION FAILED: decontaminate()`. A release
ASAN build of main reports the same `SEGV` as case 1, with this stack:
`JSReadStreamIntoSinkOperation::result()` < `rsisFinish` < `pumpOnClose`
< `sinkControllerOnClose` < `SourceHandle::cancel` <
`RewriterPipe::fail` < `RewriterPipe::abandon_suspension`. This PR
prints `{"rewrites":50,"cancelled":0}` on all three builds. Bun 1.4.2
does not crash on this script. #37108 fixed the opposite direction, a
controller destructor that reached a freed pipe.

**The freed output, case 3: `abandon_suspension()` again.** It is on
main too. The handler's promise is collected while script still holds
the `Response`, so the task is queued for a reachable rewrite. Script
drops the `Response` before the task runs. The cell is then unreachable,
but no collection has found that out, so it reads as alive. `fail()`
allocates (the error, the abort of the input), and a collection in there
sweeps the cell and both streams. `fail()` then writes the error through
`output`, a raw pointer to the freed `ByteStream`. This entry is a task
that a destructor queued, so nothing on its stack reaches the cell. It
keeps the cell that it read in a local `EnsureStillAlive` until it
returns.

```js
const encoder = new TextEncoder();
const tick = () => new Promise(resolve => setImmediate(resolve));
const responses = [], promises = [];
function start() {
  let controller;
  const input = new ReadableStream({ start: c => void (controller = c) });
  const response = new HTMLRewriter()
    .on("p", { element() { const p = new Promise(() => {}); promises.push(p); return p; } })
    .transform(new Response(input));
  response.body;
  responses.push(response);
  controller.enqueue(encoder.encode("<p>x</p>"));
}
for (let round = 0; round < 20; round++) {
  for (let i = 0; i < 4; i++) start();
  await tick();
  promises.length = 0;
  Bun.gc(true);
  responses.length = 0;
  await tick();
}
```

With `BUN_JSC_slowPathAllocsBetweenGCs=25` (a full collection at every
25th slow-path allocation) a release ASAN build of main reports the
`heap-use-after-free` in 10 of 10 runs, and also for each of 7, 13, 20,
33, 50, 64, 80, 100 and 150. This PR exits with 0. A debug build and a
release build without ASAN of main do not report it.

**The dead or freed source, case 4: `Bun.ModuleGraph.dispose()`.** It is
on main too, and needs no `onEndTag()`. `dispose()` cancels the source
of every stream that the graph's script was given (`NewSource`'s
`on_abort`, #42590), from native code. The JS wrapper owns the source,
and nothing on that stack reaches a wrapper that script has dropped.
`AbortHandleOwner` has a `KeepAlive` type for what keeps the owner alive
while `on_abort` runs. `NewSource` declared none.
- A collection that finishes inside `cancel()` frees the source under
the call, together with the rewrite that feeds it.
- After a collection that finished before, the wrapper is dead and waits
for its sweep. `cancel()` then tells the rewrite, which closes its dead
input.

`NewSource` now keeps its wrapper (`this_jsvalue.try_get()`) alive for
the call. For a wrapper that is collected and not swept yet it does
nothing: the peers are collected too, and the finalizer releases the
source. A source whose wrapper is finalized while native refs remain (a
child's pipe that nobody reads) is cancelled as before.

```js
const TICKS = 0; // or 1
const encoder = new TextEncoder();
const tick = () => new Promise(resolve => setImmediate(resolve));
const rewrite = input => new HTMLRewriter().on("div", { element() {} }).transform(input);
function start(chained) {
  for (let i = 0; i < 20; i++) {
    const input = new ReadableStream({ start: c => void c.enqueue(encoder.encode("<div>x")), cancel() {} });
    const output = rewrite(new Response(input));
    (chained ? rewrite(output) : output).body;
  }
}
for (const chained of [false, true]) {
  for (let i = 0; i < 20; i++) {
    const graph = new Bun.ModuleGraph();
    graph.run(() => start(chained));
    await tick();
    Bun.gc(false);
    for (let t = 0; t < TICKS; t++) await tick();
    graph.dispose();
  }
}
```

Run it as a file, with no options. 5 runs for each value of `TICKS`:

| build | `TICKS = 0` | `TICKS = 1` |
| --- | --- | --- |
| main, release | segfault, 5 of 5 | segfault or abort, 5 of 5 |
| main, debug | `ASSERTION FAILED: decontaminate()`, 5 of 5 | the same,
5 of 5 |
| main, release ASAN | `decontaminate()` or `SEGV`, 5 of 5 | `SEGV`, 5
of 5 |
| the version of this PR with the cell in `PipePin`, release ASAN |
`ASSERTION FAILED: result`, 5 of 5 | `SEGV` or `decontaminate()`, 5 of 5
|
| this PR, release ASAN | passes, 5 of 5 | passes, 5 of 5 |

**Each of the changes is needed.** Release ASAN builds, the repros of
cases 1 to 3:

| build | case 1 (`slowPathAllocsBetweenGCs=1`, 10 rewrites) | case 2 |
case 3 |
| --- | --- | --- | --- |
| main | `cancelled` is 0 of 10 | `SEGV` | `heap-use-after-free` |
| this PR with only the `JsRef` | `cancelled` is 0 of 10 | passes |
`heap-use-after-free` |
| this PR | 10 of 10 | passes | passes |

**Why `Element` has a back reference.** `on()` handlers find their list
through the pipe whose lol-html call is on the stack. `onEndTag()`
cannot: after an `await` in an async handler no lol-html call is on the
stack, and inside a handler of a nested, synchronous rewrite the pipe on
the stack is the inner one. Both cases work on main and have a test
here. `handler_callback` gives the pipe to the wrapper when it makes it.
`Element::invalidate()` clears it together with the lol-html pointer.
That happens when the handler returns, or when the pipe drops the
wrapper it parked, so the reference never outlives the pipe.

**Replacement.** A second `onEndTag()` on the same element replaces the
first callback, in one handler or across two `on()` handlers. That is
the behavior of main (`handlers.clear()` then push). lol-html gives
every matching handler the same `Element`, and a suspension moves the
user data with the parked copy. So the first call pushes the one
lol-html handler and records its slot as the element's user data, and a
later call only stores the new callback into that slot.

**A parked element that outlives its transform cell.** A handler returns
a promise, the element is parked, then the promise and the cell are
collected together. Until the queued `abandon_suspension` task runs,
script can still call `onEndTag()` on the parked element. The cell is
then swept, or dead and not swept yet. In both states `try_get()` is
`None`, and `hold_end_tag_callback` stores nothing. It also stores
nothing for a parked element whose rewrite was cancelled or failed. In
all of these the end tag can never come. The last test in the file
covers the collected case.

**Return value.** Unchanged: the element while it is attached, `null`
once it is detached.

**Sizes.** `size_of` in the release profile, merge base then this PR:
`Element` 40, 48 (one per element handler call, freed when the handler
returns). `RewriterPipe` 272, 304 (one per `transform()`, alignment 16).
`EndTagHandler` 24, 16. The transform cell gets one more `WriteBarrier`
(8 bytes). The first `onEndTag()` call on an element now boxes a 4-byte
index as the lol-html user data, and no longer adds an entry to the VM's
table of protected values. `size` of the linux-x64 release binaries:
text 80,681,674, then 80,690,634. Data and bss do not change.

**Speed.** Release builds of the merge base and of this PR, linux-x64,
pinned to one core. One process rewrites a document of 20,000 elements
with one element handler, and reports the best of 40 runs in nanoseconds
per element. 25 rounds run the processes in turn: base, PR, base again.
The table gives the minimum of the rounds, and the median in
parentheses. The third column is the same base binary, which shows the
noise.

| case | base | this PR | base again |
| --- | --- | --- | --- |
| `<p>x</p>` x 20,000, handler only | 150.1 (154.9) | 150.9 (155.7) |
151.3 (157.7) |
| `<p>x</p>` x 20,000, handler calls `onEndTag()` | 298.3 (306.2) |
257.6 (266.8) | 298.0 (306.7) |
| `<div>` nested 50 deep x 400, handler only | 150.1 (154.2) | 150.5
(154.4) | 150.1 (154.5) |
| `<div>` nested 50 deep x 400, handler calls `onEndTag()` | 322.1
(331.4) | 274.6 (280.6) | 326.8 (332.7) |

`try_get()` is one call into C++ per handler call. The version of this
PR without it measured 151.0, 259.8, 151.9 and 273.8 in the same rounds.

**Not changed.** Each of these keeps a rewrite alive on main and on this
PR with no `onEndTag()` call at all (100 rewrites each):
- A failed rewrite keeps the handler's error in the output body as a
`Strong` until the body is read (`fail()`). If that error references the
output `Response` (for example `error.response = res`) and nothing reads
the body, 100 of 100 Responses stay. With the body read, 0 stay. An
error that does not reference the Response retains nothing.
- A `read()` that is pending on the output while the JS input stream can
never produce again keeps 100 of 100 Responses, through the protected
promise and buffer of the pending pull. A pending `.text()` leaves 100
protected Promises (`promise_value.protect()`,
`src/runtime/webcore/Body.rs:453`).
- A rewrite that fails or is cancelled keeps its lol-html rewriter (the
parser arena) until the transform cell is collected. Only `finish()`
frees it earlier. A parked wrapper can still point into it, so an
earlier free needs its own change.

**Tests.** 23 new tests. On a debug build of main 11 fail: the three
leak shapes, four of the five kept-Response shapes, the parked rewrite
that dies with its handler promise, the two `dispose()` tests of case 4,
and the ModuleGraph test. On a release ASAN build of main the cancel
test under continuous collection and the test of case 3 fail too. The
other tests guard the new storage and pass before the fix: the callback
stays alive until its end tag with nothing else referencing it (GC churn
in between, half of the outputs dropped), slot reuse under nesting, one
end tag that closes several elements, callbacks that run after a handler
cancelled the output earlier in the same chunk, replacement (one
handler, two handlers, across a suspension), `onEndTag()` after an
`await`, an outer element used in a nested rewrite, an indexed accessor
on `Array.prototype`, the parked element whose rewrite was collected,
and the kept-Response case of a rewrite that completes.

**Suites run on the debug (ASAN) build:**
`test/js/workerd/html-rewriter-leak.test.ts` (49 pass, 1 skipped in
debug), `test/js/bun/module-graph/module-graph-gc.test.ts` (33 pass),
`test/js/workerd/html-rewriter.test.js` (186 pass),
`test/js/workerd/html-rewriter-end-error.test.ts`,
`test/js/web/html/html-rewriter-doctype.test.ts`,
`test/regression/issue/htmlrewriter-additional-bugs.test.ts`,
`test/regression/issue/text-chunk-null-access.test.ts`,
`test/js/bun/http/serve-stream-body-error.test.ts`, and the HTMLRewriter
cases of `test/js/web/workers/worker-terminate-lifetime.test.ts` and
`test/js/bun/module-graph/module-graph-isolation.test.ts`. On the
release ASAN build: the leak file with the CI environment of the ASAN
lane (`BUN_JSC_validateExceptionChecks=1`,
`BUN_GARBAGE_COLLECTOR_LEVEL=1`) and `html-rewriter.test.js`, 3 runs.
Also run by hand, on the earlier version with the cell in `PipePin`: a
nested document with `Bun.gc(true)` in handlers and callbacks under
`BUN_JSC_collectContinuously=1`, and `terminate()` of workers that hold
pending callbacks in each state.

**A test of this file that is slow on main.** "HTMLRewriter does not
leak element/document handler allocations" passes 15 s on a loaded
release ASAN build, on main and on this PR. #44359 and #44377 change
that test.

**Self-review.** The review of the first commit asked for the
ModuleGraph test, for the narrower wording of the measured shapes, and
for the "Not changed" list. All three are in.

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/bun/module-graph/module-graph-gc.test.ts

<!-- robobun:evidence:end -->

---------

Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>